Repository navigation
refactor(torch-mocker): align HIP streams and remove torch dependencies - #4
Merged
Merged
Conversation
There was a problem hiding this comment.
AI Review
🟡 建议修改
发现有明确证据的问题,建议核对并处理。
已确认 1 个问题,其中 0 个已添加到对应代码行。
变更概览
本次变更主要包含:
- 移除 torch-mocker 构建对 HIP torch 源码目录 HIP_TORCH_PATH 的依赖:setup.py 改为自动从导入的 torch 推导 TORCH_PATH 并修正异常抛出方式,README 同步更新构建与 CTest 说明,测试目录改为 torch-mocker/build
- 为 torch-mocker 构建新增 TORCH_PATH 缓存变量与自动探测逻辑(缓存、环境变量或经 Python 定位已安装 torch 包位置),校验 include 目录存在,并把原 $ENV{TORCH_PATH} 的头文件与库路径引用统一替换为 CMake 变量
- 取消 torch_cuda 对 libtorch_python.so 及 Python 库的链接,并移除 $ENV{HIP_TORCH_PATH} 相关的 include 指令
- 本批将 torch-mocker/c10/util/StringUtil.cpp 由通过 #include 方式引用 PyTorch 源码改为自包含实现,内联定义了 StripBasename、ReplaceAll、tryToNumber、split 等 c10 字符串工具函数,以解除对 PyTorch 源文件的编译依赖。
文件审查摘要
| 文件 | 变更 | 审查结果 |
|---|---|---|
torch-mocker/c10/util/flags_use_no_gflags.cpp |
修改 · +206/-1 | P3 × 1 |
torch-mocker/torch/csrc/cuda/Stream.cpp |
删除 · +0/-236 | — |
torch-mocker/c10/util/StringUtil.cpp |
修改 · +218/-1 | — |
torch-mocker/include/torch/csrc/jit/cuda/cuda.h |
修改 · +38/-34 | — |
torch-mocker/test_py/test_python_c.cpp |
删除 · +0/-72 | — |
torch-mocker/include/torch/csrc/cuda/nccl.h |
修改 · +4/-58 | — |
torch-mocker/CMakeLists.txt |
修改 · +41/-15 | — |
README.md |
修改 · +10/-14 | — |
torch-mocker/include/torch/csrc/autograd/functions/comm.h |
修改 · +2/-16 | — |
torch-mocker/setup.py |
修改 · +8/-9 | — |
setup.py |
修改 · +7/-7 | — |
torch-mocker/include/torch/csrc/distributed/c10d/symm_mem/intra_node_comm.hpp |
修改 · +1/-11 | — |
torch-mocker/include/torch/csrc/cuda/comm.h |
修改 · +0/-10 | — |
torch-mocker/test_py/CMakeLists.txt |
修改 · +4/-4 | — |
torch-mocker/tests/CMakeLists.txt |
修改 · +4/-4 | — |
torch-mocker/include/torch/csrc/cuda/Stream.h |
修改 · +1/-6 | — |
torch-mocker/include/torch/csrc/cuda/utils.h |
修改 · +1/-5 | — |
torch-mocker/include/torch/csrc/cuda/CUDAPluggableAllocator.h |
修改 · +1/-1 | — |
torch-mocker/torch/share/cmake/Caffe2/Caffe2Targets.cmake |
修改 · +1/-1 | — |
无法定位到 diff 行的问题(1)
P3 · 数值类型解析的错误处理路径不可达,非法输入被静默解析为 0
- 文件:
torch-mocker/c10/util/flags_use_no_gflags.cpp(无法安全定位到当前 diff 行) - 触发条件:向 int、int64_t 或 double 类型的 flag 传入无法转换为数字的值(如 --x=abc)。
- 证据与影响:Parse<int>、Parse<int64_t>、Parse<double> 中使用 std::atoi/std::atoll/std::atof 并包裹 try/catch,但这些函数按标准不会抛出异常,非法输入返回 0,导致 catch 中的错误提示与 return false 永不执行。 非法的数值型 flag 值被静默当作 0 且解析返回 success,配置错误难以察觉和排查。
- 修改建议:改用 std::strtol/strtoll/strtod,通过检查 endptr 与 errno 判断转换失败并返回 false。
审查信息
- 变更统计:19 个文件,+547/-505。
- 覆盖情况:共 19 个文件,已完整审查 19 个。
- 候选问题:1 项;证据复核过滤:0 项;发布前敏感信息保护:0 项。
- 本服务以 GitHub 提供的 PR diff 为审查主体,PR 描述与按相关性选取的仓库片段仅用于核验;未执行代码或重跑测试,结论仍需维护者核验。
wenzhs
approved these changes
Oct 10, 2026
There was a problem hiding this comment.
AI Review
🟢 未发现有明确证据的问题
在已提供并完成审查的 diff 范围内未形成可确认问题。
变更概览
本次变更主要包含:
- setup.py 不再要求手动设置 HIP_TORCH_PATH 与 TORCH_PATH 环境变量,改为从导入的 torch 包路径自动推导 TORCH_PATH 供 CMake 使用,并将 raise 字符串修正为抛出 RuntimeError 异常;README 同步更新了构建与 CTest 运行步骤。
- 本批将 torch-mocker 构建改为自动探测已安装 Torch 包路径(缓存变量、环境变量、Python 导入三级来源),统一用 TORCH_PATH 变量替代原环境变量引用,并取消 libtorch_cuda 对 libtorch_python 与 Python 库的链接。
- 在 torch-mocker 中新增 c10/util/StringUtil.cpp 的本地实现,将原先直接包含 PyTorch 源文件的写法改为包含对应头文件,去除对 PyTorch 源码的依赖
- 为 torch-mocker 新增独立的 c10 字符串工具实现:包含就地/缓冲区两种子串替换、基于 strtoll/strtod 的字符串转数值函数(各提供 std::string 与 const char* 特化)以及按单字符分隔符切分的 split 函数,用于替代对 PyTorch 源码中相应工具的依赖。
文件审查摘要
| 文件 | 变更 | 审查结果 |
|---|---|---|
torch-mocker/torch/csrc/cuda/Stream.cpp |
删除 · +0/-236 | — |
torch-mocker/c10/util/StringUtil.cpp |
修改 · +218/-1 | — |
torch-mocker/c10/util/flags_use_no_gflags.cpp |
修改 · +206/-1 | — |
torch-mocker/include/torch/csrc/jit/cuda/cuda.h |
修改 · +38/-34 | — |
torch-mocker/test_py/test_python_c.cpp |
删除 · +0/-72 | — |
torch-mocker/include/torch/csrc/cuda/nccl.h |
修改 · +4/-58 | — |
torch-mocker/CMakeLists.txt |
修改 · +41/-15 | — |
README.md |
修改 · +10/-14 | — |
torch-mocker/include/torch/csrc/autograd/functions/comm.h |
修改 · +2/-16 | — |
torch-mocker/setup.py |
修改 · +8/-9 | — |
setup.py |
修改 · +7/-7 | — |
torch-mocker/include/torch/csrc/distributed/c10d/symm_mem/intra_node_comm.hpp |
修改 · +1/-11 | — |
torch-mocker/include/torch/csrc/cuda/comm.h |
修改 · +0/-10 | — |
torch-mocker/test_py/CMakeLists.txt |
修改 · +4/-4 | — |
torch-mocker/tests/CMakeLists.txt |
修改 · +4/-4 | — |
torch-mocker/include/torch/csrc/cuda/Stream.h |
修改 · +1/-6 | — |
torch-mocker/include/torch/csrc/cuda/utils.h |
修改 · +1/-5 | — |
torch-mocker/include/torch/csrc/cuda/CUDAPluggableAllocator.h |
修改 · +1/-1 | — |
torch-mocker/torch/share/cmake/Caffe2/Caffe2Targets.cmake |
修改 · +1/-1 | — |
审查信息
- 变更统计:19 个文件,+547/-505。
- 覆盖情况:共 19 个文件,已完整审查 19 个。
- 候选问题:0 项;证据复核过滤:0 项;发布前敏感信息保护:0 项。
- 本服务以 GitHub 提供的 PR diff 为审查主体,PR 描述与按相关性选取的仓库片段仅用于核验;未执行代码或重跑测试,结论仍需维护者核验。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
torch-mocker.libtorch_cuda.so.Validation